Skip to content

Conversation

@MaxEW707
Copy link
Contributor

Fixes #87717.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels Oct 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 12, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: Max Winkler (MaxEW707)

Changes

Fixes #87717.


Full diff: https://github.com/llvm/llvm-project/pull/112066.diff

1 Files Affected:

  • (modified) clang/lib/Headers/intrin0.h (+1-1)
diff --git a/clang/lib/Headers/intrin0.h b/clang/lib/Headers/intrin0.h
index 866c8896617d22..6b01f3808652aa 100644
--- a/clang/lib/Headers/intrin0.h
+++ b/clang/lib/Headers/intrin0.h
@@ -44,7 +44,7 @@ unsigned char _InterlockedCompareExchange128_rel(__int64 volatile *_Destination,
                                                  __int64 *_ComparandResult);
 #endif
 
-#ifdef __x86_64__ && !defined(__arm64ec__)
+#if defined(__x86_64__) && !defined(__arm64ec__)
 unsigned __int64 _umul128(unsigned __int64, unsigned __int64,
                           unsigned __int64 *);
 unsigned __int64 __shiftleft128(unsigned __int64 _LowPart,

@efriedma-quic
Copy link
Collaborator

Please add a testcase (something similar to clang/test/Headers/ms-intrin.cpp, but including intrin0.h instead of intrin.h, to catch obvious mistakes in the header).

Not sure how you're actually hitting the error you reported; the clang header directory should count as a system header, so the warning should normally be suppressed, I think.

@MaxEW707
Copy link
Contributor Author

Not sure how you're actually hitting the error you reported; the clang header directory should count as a system header, so the warning should normally be suppressed, I think.

It is counted as a system header and I could not repro the warning in my local tests. I was going off the user who posted the issue in the previous PR.

clang_cc1 substitution in lit also adds the header path via -internal-isystem which is why our unit tests weren't hitting this.

Please add a testcase (something similar to clang/test/Headers/ms-intrin.cpp, but including intrin0.h instead of intrin.h, to catch obvious mistakes in the header).

intrin.h includes intrin0.h. Our unit tests weren't hitting this since lit adds the clang header path as a system include so warnings are suppressed. I should be able to use "--no-system-header-prefix" to workaround that. I'll give that a try inside ms-intrin.cpp and that will cover both intrin.h and intrin0.h.

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MaxEW707 MaxEW707 merged commit 9bf68c2 into llvm:main Oct 14, 2024
8 checks passed
@MaxEW707 MaxEW707 deleted the mew/fix-intrin0-extra-tokens branch October 14, 2024 19:22
@MaxEW707
Copy link
Contributor Author

@efriedma-quic What is the process for getting a fix into a milestone such as the upcoming 19.1.2 milestone. I read the docs on cherry-pick but I couldn't find any information about who decides what is considered suitable for a patch release.
I am going to defer to you if you think this is worthwhile for 19.1.2 and if so what are the next steps to make that happen.

@mstorsjo
Copy link
Member

I think it's backport worthy. It's very small and has a near zero regression risk, and is a quite obvious fix. It might be a bit late for 19.1.2 which should be cut tomorrow, but should probably make it for 19.1.3.

Just add this PR to the right 19.x milestone and type a /cherry-pick comment here with the commit hash, and the bots should do the rest.

@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 14, 2024

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building clang at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/6543

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-unit :: Utility/./UtilityTests/5/8 (2029 of 2038)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests/1/3 (2030 of 2038)
PASS: lldb-unit :: ValueObject/./LLDBValueObjectTests/2/3 (2031 of 2038)
PASS: lldb-unit :: tools/lldb-server/tests/./LLDBServerTests/0/2 (2032 of 2038)
PASS: lldb-unit :: tools/lldb-server/tests/./LLDBServerTests/1/2 (2033 of 2038)
PASS: lldb-unit :: Host/./HostTests/11/12 (2034 of 2038)
PASS: lldb-unit :: Target/./TargetTests/11/14 (2035 of 2038)
PASS: lldb-unit :: Host/./HostTests/3/12 (2036 of 2038)
PASS: lldb-unit :: Process/gdb-remote/./ProcessGdbRemoteTests/8/9 (2037 of 2038)
UNRESOLVED: lldb-api :: tools/lldb-server/TestLldbGdbServer.py (2038 of 2038)
******************** TEST 'lldb-api :: tools/lldb-server/TestLldbGdbServer.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/local/bin/llvm-ar --env OBJCOPY=/usr/bin/llvm-objcopy --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --arch aarch64 --build-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/dsymutil --make /usr/bin/make --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/tools/lldb-server -p TestLldbGdbServer.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 9bf68c2400e8966511332dfbf5c0f05e8a3300fa)
  clang revision 9bf68c2400e8966511332dfbf5c0f05e8a3300fa
  llvm revision 9bf68c2400e8966511332dfbf5c0f05e8a3300fa
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hc_then_Csignal_signals_correct_thread_launch_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hc_then_Csignal_signals_correct_thread_launch_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_fails_on_another_pid_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_fails_on_minus_one_pid_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_fails_on_zero_pid_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_switches_to_3_threads_launch_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_Hg_switches_to_3_threads_launch_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_P_and_p_thread_suffix_work_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_P_and_p_thread_suffix_work_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_P_writes_all_gpr_registers_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
PASS: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_P_writes_all_gpr_registers_llgs (TestLldbGdbServer.LldbGdbServerTestCase)
UNSUPPORTED: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_attach_commandline_continue_app_exits_debugserver (TestLldbGdbServer.LldbGdbServerTestCase) (test case does not fall in any category of interest for this run) 
Program aborted due to an unhandled Error:
Operation not permitted
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb-server gdbserver --attach=1715043 --reverse-connect [127.0.0.1]:36935
 #0 0x0000aaaab0240b2c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb-server+0xb90b2c)
 #1 0x0000aaaab023eb5c llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb-server+0xb8eb5c)
 #2 0x0000aaaab024123c SignalHandler(int) Signals.cpp:0:0
 #3 0x0000ffffa2f447dc (linux-vdso.so.1+0x7dc)
 #4 0x0000ffffa274f200 __pthread_kill_implementation ./nptl/pthread_kill.c:44:76

@llvmbot
Copy link
Member

llvmbot commented Oct 14, 2024

I think it's backport worthy. It's very small and has a near zero regression risk, and is a quite obvious fix. It might be a bit late for 19.1.2 which should be cut tomorrow, but should probably make it for 19.1.3.

Just add this PR to the right 19.x milestone and type a /cherry-pick comment here with the commit hash, and the bots should do the rest.

Error: Command failed due to missing milestone.

@efriedma-quic efriedma-quic added this to the LLVM 19.X Release milestone Oct 14, 2024
@efriedma-quic
Copy link
Collaborator

/cherry-pick 9bf68c2

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 14, 2024

/pull-request #112258

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 15, 2024
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

5 participants